Skip to content

feat(settings): add opt-in atomic write mode - #29

Merged
Kibnet merged 1 commit into
masterfrom
fix/settings-file-crash-safety
Sep 21, 2026
Merged

Kibnet merged 1 commit into
masterfrom
fix/settings-file-crash-safety

Conversation

@Kibnet

@Kibnet Kibnet commented Sep 21, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add an opt-in atomic write mode for Windows settings files.
  • Prepare and publish WritableJsonConfiguration 8.1.0 after merge.

Changes

  • Serialize in-process writes per physical path and stage in-memory state until commit.
  • Write a validated temporary file, preserve restrictive ACLs before settings bytes, flush to disk, atomically replace the main file, and retain one readable backup.
  • Reconcile ambiguous failures with disk and block later writes when reconciliation is impossible.
  • Preserve the existing behavior by default; non-Windows explicit opt-in fails before writing.
  • Add crash/fault/concurrency/ACL regression coverage, package README/license/release notes, and CHANGELOG.md.
  • Make CI build and test pull requests; package publishing now runs only after a push to master.

Validation

  • dotnet build WritableJsonConfiguration.sln -c Release --no-restore — 0 errors (2 pre-existing nullable warnings in ConfigTests).
  • dotnet test WritableJsonConfiguration.sln -c Release --no-build --no-restore — 38 passed, 1 expected non-Windows contract skip on Windows.
  • Packed WritableJsonConfiguration.8.1.0.nupkg; verified version, MIT license, README, release notes, .NET Standard 2.0 library and dependencies.
  • Clean consumer restored the local 8.1.0 package, built with 0 errors, and exercised two atomic writes plus backup/read-back successfully.
  • Package SHA-256: A68384F2DC328DA97E0EF8904068577F0D76CA15808408190DFF3E092846F6ED (local candidate; CI rebuild will have its own deterministic delivery artifact).
  • UI evidence: not applicable; this repository is a configuration library with no UI.

Risks / Rollback

  • Atomic mode is Windows-only and disabled by default, so existing consumers retain legacy behavior.
  • The mode does not coordinate multiple processes or provide arbitrary hardware-failure guarantees.
  • NuGet packages are immutable. Before merge, rollback is closing the PR; after publication, any correction requires a new patch version.
  • The workflow must pass on .NET SDK 8 before merge; local validation used stable SDK 10.0.401 because SDK 8 is not installed locally.

Links

  • Consumer incident/spec: Unlimotion settings crash-safety work (local approved SPEC; not published from this repository).

Add serialized Windows atomic replacement with backup, staged in-memory publication, permission preservation, failure reconciliation, crash tests, release metadata, and gated package publishing.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-21T11:56:00.949583Z 42874f3 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Kibnet
Kibnet merged commit 4e4157d into master Sep 21, 2026
1 check passed
@Kibnet
Kibnet deleted the fix/settings-file-crash-safety branch September 21, 2026 11:52

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 42874f354d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +51 to +55
if (!publishData)
{
// Atomic mode must preserve siblings in the actual parent, not recreate
// nested containers by looking for their names at the document root.
context = GetOrCreateAtomicChild((JToken)context, currentKey, int.TryParse(split[i + 1], out _));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve numeric keys when the parent is an object

When atomic mode updates a configuration such as {"Years":{"2024":"old"}} through configuration["Years:2024"] = "new", this branch routes the mutation through the JSON navigator without calling base.Set. The final segment is classified as an array index solely because it parses as an integer; since its parent is a JObject, no assignment occurs, the operation returns successfully as a no-op, and both memory and disk retain old. Numeric object keys are valid and commonly produced by dictionaries, so index handling needs to depend on the parent token type rather than the key text alone.

Useful? React with 👍 / 👎.

Comment on lines +175 to +179
if (index < array.Count) return array[index];
JToken child = nextIsArrayIndex ? (JToken)new JArray() : new JObject();
// Preserve the existing append/merge behavior rather than truncating array tails.
array.Add(child);
return child;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject or preserve the requested sparse array index

For a nested write past an array's tail, such as setting Items:2:Name when Items is empty, this code appends exactly one child at index 0 and then applies the edit there. The save therefore succeeds but changes Items:0:Name, while the requested Items:2:Name remains absent after the candidate data is reparsed. Either pad the array through the requested index or reject sparse indices rather than silently writing to a different configuration key.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant